Add Gemini buddy example - #190
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new development example, gemini-buddy-mcp, which implements an MCP service wrapping the Gemini CLI to maintain multi-turn conversation state server-side. The feedback highlights several critical areas for improvement: resolving potential character corruption by explicitly setting UTF-8 encoding on child process streams, avoiding the security risk of hardcoding API keys in the Dockerfile, sanitizing user messages containing newlines to preserve the transcript structure, and addressing potential race conditions from concurrent requests on the in-memory session store.
| let stdout = ""; | ||
| let stderr = ""; | ||
| child.stdout.on("data", (d) => { stdout += d; }); | ||
| child.stderr.on("data", (d) => { stderr += d; }); |
There was a problem hiding this comment.
When reading from child.stdout and child.stderr, the data chunks are emitted as Buffer objects. Concatenating them directly to a string (stdout += d) implicitly converts each buffer chunk to a string. If a multi-byte character (such as an emoji or non-ASCII character) is split across chunk boundaries, this implicit conversion will result in corrupted characters (e.g., ``).
To prevent this, set the encoding of the streams to 'utf8' before listening to the 'data' event, which ensures that multi-byte characters are decoded correctly across chunk boundaries.
| let stdout = ""; | |
| let stderr = ""; | |
| child.stdout.on("data", (d) => { stdout += d; }); | |
| child.stderr.on("data", (d) => { stderr += d; }); | |
| let stdout = ""; | |
| let stderr = ""; | |
| child.stdout.setEncoding("utf8"); | |
| child.stderr.setEncoding("utf8"); | |
| child.stdout.on("data", (d) => { stdout += d; }); | |
| child.stderr.on("data", (d) => { stderr += d; }); |
| function render(turns) { | ||
| return turns.map((t) => `${t.role === "user" ? "User" : "Buddy"}: ${t.text}`).join("\n"); | ||
| } |
There was a problem hiding this comment.
If a user message contains newlines, the rendered transcript will contain lines that are not prefixed with User: or Buddy:. This violates the assumption in the system prompt (You'll get the conversation so far on stdin, lines prefixed 'User:' and 'Buddy:') and can confuse the model.
Consider sanitizing or replacing newlines in the message text to keep the transcript structure clean and predictable.
function render(turns) {
return turns.map((t) => {
const role = t.role === "user" ? "User" : "Buddy";
const cleanText = t.text.replace(/\\r?\\n/g, " ");
return `${role}: ${cleanText}`;
}).join("\\n");
}| async ({ message, session_id }) => { | ||
| const id = session_id ?? "default"; | ||
| const turns = transcript(id); | ||
| turns.push({ role: "user", text: message }); | ||
| try { | ||
| const reply = await runGemini(render(turns)); | ||
| turns.push({ role: "assistant", text: reply }); | ||
| return { content: [{ type: "text", text: reply }] }; | ||
| } catch (err) { | ||
| turns.pop(); // drop the turn we couldn't answer so it can be retried | ||
| return { content: [{ type: "text", text: String(err?.message ?? err) }], isError: true }; | ||
| } | ||
| }, |
There was a problem hiding this comment.
Since the server handles requests asynchronously and stores session history in a shared in-memory sessions Map, concurrent requests for the same session_id can lead to race conditions. If two chat requests for the same session are processed concurrently, both will read the same initial transcript, append their respective messages, and call runGemini in parallel. This can result in corrupted history, duplicate turns, or out-of-order messages.
While this is a development example, it is worth noting that in a production environment, you should serialize requests per session (e.g., using a promise chain or queue per session ID) or use a persistent, transactional database.
No description provided.